[v2] Bound in-process FFI host_shutdown and fix .NET lifecycle race across all SDKs by SteveSandersonMS · Pull Request #2531 · github/copilot-sdk

SteveSandersonMS · 2026-09-04T10:16:11Z

Summary

Closes #2525 — the remaining lifecycle/reliability work carried forward from
the superseded FFI tracker #1934 (parent tracker: #2522).

This PR is scoped to the items #2525 assigns to the SDK: Windows in-process
CI, callback/teardown reliability, forceStop/session-disposal resource
cleanup, SQLite locking/closure behavior, graceful-shutdown performance, and
platform test exclusions. It intentionally does not touch artifact
acquisition/embedding (#2524) or the public option redesign (#2523).

Latest status: rebased onto main at 51cf90df (includes Copilot CLI/runtime
1.0.84-4 and #2610's callback-reclamation fix), then reran the full SDK CI.
All SDK build/test jobs are green with Rust/.NET Windows-in-process still
excluded; CodeQL C# is still running at the time of this update.

What was verified as already resolved (no change needed)

napi-oop is confirmed gone. Per the maintainer's comment on [v2] Complete in-process lifecycle and platform reliability work #2525,
napi-oop is no longer used by the runtime, so PR Fix Windows in-process Node E2E cleanup hang #1983's "napi-oop peer
shutdown crash" concern and [Tracking] In-process (FFI) items to be cleaned up #1934's napi-oop-flavored SQLite concerns are
moot as a root cause. (The string napi-oop-runtime remaining in the repo
is only an artifact-bundling exclusion directory name, not a live
dependency.)
Node.js SDK's Windows in-process CI matrix already runs with no
exclusion, and continues to pass reliably in this PR's CI runs.
Java SDK already has full Windows in-process CI (windows-latest and
windows-11-arm in the java-sdk-inprocess job), per [WIP] Achieve 100% InProcess and out-of-process test parity #2272.
Go and Python SDKs already run the full [ubuntu-latest, macos-latest, windows-latest] matrix for both default and inprocess transports, with
no Windows-specific exclusions, and continue to pass reliably in this PR's
CI runs.

So the CI-level gap from #1934/#2525 was narrower than the issue's title
suggests: only Rust and .NET still excluded Windows-in-process from
CI.

Root cause #1 — fixed by SDK/runtime split after rebase

The original SDK-owned bug was that every in-process FFI host's dispose/close
path called the native host_shutdown export synchronously with no timeout.
A stuck or slow native shutdown — the "SQLite file locking on Windows"
failure mode called out in #1934 and #2525 — could therefore hang graceful
stop and, worse, hang the documented forceStop/force_stop recovery path.

This branch bounds host_shutdown with a 10s wait in all five in-process SDKs
and adds bounded-time regression coverage. After rebasing, this is layered on
top of #2610's newer callback-reclamation work from main: callback state is
only released once connection_close reports quiescence, then host shutdown
is run with a bounded wait.

Files touched:

dotnet/src/FfiRuntimeHost.cs
nodejs/src/ffiRuntimeHost.ts
rust/src/ffi.rs
python/copilot/_ffi_runtime_host.py
go/internal/ffihost/ffihost.go

Root cause #2 — fixed (.NET-specific lifecycle race)

Bounding .NET's Dispose() wait introduced a narrower race: if native
shutdown is genuinely slow, Dispose() now returns before shutdown is done,
leaving it running on a background task. A subsequent StartAsync() on a new
client instance could then overlap host_start/connection_open with that
still-draining shutdown.

Fixed with a static SemaphoreSlim gate in FfiRuntimeHost.cs that
serializes native start/open against host shutdown, held for the true duration
of shutdown (even past the point Dispose() stops waiting), without blocking
already-live connections from running concurrently.

Root cause #3 — confirmed still NOT fixed by the newer runtime

I retried the exact Rust/.NET Windows-in-process CI cells after rebasing onto
latest main, which now includes Copilot CLI/runtime 1.0.84-4 and additional
in-process E2E stabilization work. The blocker still reproduces:

.NET Windows in-process: all three retried non-CAPI cells failed again;
one reproduced the same System.AccessViolationException in
NativeConnectionWrite, while the others crashed/aborted or timed out in
the same Windows in-process run.
Rust Windows in-process: the job still failed when enabled on the newer
runtime; the run also exposed a separate Rust Windows lifecycle fixture bug
(fixed in this PR by skipping the stdio-only job-object test for in-process
runs and waiting for a parseable PID file instead of merely file existence).

The original cross-language conclusion remains valid: two independent FFI
binding implementations (.NET P/Invoke and Rust extern "C"/libloading) hit
the same native crash class on Windows, while Node/Go/Python Windows
in-process cells keep passing. This is strong evidence of a bug in the shared
native runtime cdylib, not in either SDK binding.

Filed upstream: github/copilot-agent-runtime#18990.

Decision: Rust/.NET Windows-in-process CI remains excluded pending that
runtime fix. Re-enabling those cells now would make unrelated SDK PRs red for
a blocker this repo cannot fix.

CI/workflow fixes kept

Rust default test job timeout is raised above its own step-level timeout so
a slow-but-healthy Windows compile is not canceled early.
.NET test job timeout is raised from 20 to 30 minutes after the newer
runtime/base made macOS default shard 1 exceed the old bound while all other
shards passed.
Rust Windows lifecycle fixture hardened: the stdio job-object containment
test is skipped under in-process transport, and the PID-file waiter now waits
for parseable PID content rather than file existence.

Tests

Added or preserved bounded-time regression coverage in every SDK asserting
forceStop/force_stop/Dispose completes within a bounded time instead of
hanging:

.NET: Should_Force_Stop_Over_InProcess_Ffi_Within_Bounded_Time (E2E)
Node.js: should stop within a bounded time over the in-process transport (E2E)
Rust: should_force_stop_inprocess_client_within_bounded_time (E2E)
Python: test_should_force_stop_over_in_process_ffi_within_bounded_time (E2E)
Go: deterministic stuck-shutdown unit coverage plus the in-process E2E

Latest local validation after rebase/conflict resolution:

.NET: dotnet build --no-restore
Node.js: npm run -s build and npm test -- --run test/ffiRuntimeHost.test.ts
Rust: cargo check --lib --features bundled-in-process,test-support (rerun
with CARGO_BUILD_JOBS=1 after an earlier local SIGKILL from resource
pressure)
Python: python3 -m py_compile python/copilot/_ffi_runtime_host.py
Workflow YAML parse check for .NET/Rust workflows

Out of scope / follow-ups

github/copilot-agent-runtime#18990: native Windows in-process crash in
the runtime cdylib. Blocks re-enabling Rust/.NET Windows-in-process CI and
re-measuring shutdown performance there.
[v2] Consolidate runtime discovery, acquisition, and embedding #2524 (artifact acquisition/embedding) and [v2] Redesign process and transport configuration APIs #2523 (public option redesign)
are untouched, per this issue's explicit scope boundaries.

github-actions

Generated by SDK Consistency Review Agent for #2531 · copilot · sonnet50 · 27 AIC · ⌖ 12.1 AIC · ⊞ 9.7K

github-actions

Generated by SDK Consistency Review Agent for #2531 · copilot · sonnet50 · 41.9 AIC · ⌖ 12.2 AIC · ⊞ 9.7K

github-actions

Generated by SDK Consistency Review Agent for #2531 · copilot · sonnet50 · 43 AIC · ⌖ 12.1 AIC · ⊞ 9.7K

… in-process CI Addresses #2525, the remaining lifecycle/reliability work carried forward from the superseded FFI tracker #1934. Root cause fixed (SDK-owned, all five in-process SDKs): Each in-process FFI host's dispose/close path called the native `host_shutdown` export synchronously with no timeout: - .NET: `FfiRuntimeHost.Dispose()` - Node.js: `FfiRuntimeHost.dispose()` (worst case: blocked the entire single-threaded event loop, not just one continuation) - Rust: `FfiShared::close()`, called from `Client::force_stop()`, which is explicitly documented as a synchronous, infallible recovery path for a hung/slow `stop()` -- defeating its own contract - Python: `FfiRuntimeHost.dispose()`, called synchronously from async `force_stop()`, blocking the whole event loop - Go: `Host.Dispose()`, called from `Client.ForceStop()`, documented the same way as Rust's `force_stop` A stuck or slow native shutdown (the exact "SQLite file locking on Windows" failure mode called out in #1934/#2525) could therefore hang graceful stop, and worse, hang the documented forceStop/force_stop recovery path meant to rescue callers from exactly that hang. Fix, applied consistently across all five SDKs: run the native call on a background thread/task/goroutine, bound the wait with a 10s timeout, and defer freeing the associated callback handle/state until the native call actually completes (never on the timeout path), so an abandoned call can't later invoke a freed callback. If the bound elapses, log a warning and return without joining further; the background thread/task continues running the real shutdown to completion. CI: - Rust: `napi-oop` is confirmed gone (per maintainer comment on #2525), so removed the stale "napi-oop peer shutdown crash" TODO and re-enabled windows-latest in the test-inprocess matrix. - .NET: removed the blanket Windows+inprocess exclusion (the underlying concern is now bounded by the Dispose fix); kept the existing, unrelated, already-tracked CAPI-in-process regression exclusion (TODO(cli-1.0.81-2)) scoped only to that backend, and added new Windows in-process include cells for the other backends, mirroring the existing Linux cells. - Confirmed Java, Go, and Python already have full Windows in-process CI coverage with no exclusions; no workflow changes needed for those SDKs. Tests: added regression coverage in each SDK asserting force-stop/dispose completes within a bounded time instead of hanging (.NET/Node/Python/Rust E2E against a live in-process runtime; Go unit test using a mocked stuck native call to deterministically exercise the timeout path without CI flakiness). Not in scope here (runtime-owned, github/copilot-agent-runtime): the native `host_shutdown` implementation itself, including its SQLite session store closing behavior. The SDK-side bound prevents hangs regardless of how slow or buggy that implementation is, but does not by itself fix a slow/buggy native shutdown -- see PR description for the linked follow-up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…indows CI Real Windows in-process CI evidence from the first PR run surfaced two distinct, previously-latent issues (both invisible before because Windows in-process was excluded entirely): 1. dotnet: `Dispose_Disconnects_Client_And_Disposes_Rpc_Surface` crashed the whole test host with `System.AccessViolationException` inside `ConnectionWrite`/`NativeConnectionWrite` while a *different* client was still handshaking. Bounding `Dispose()`'s wait on `host_shutdown` (previous commit) means a slow shutdown can still be draining on an abandoned background thread after `Dispose()` already returned to its caller; the next client's `StartAsync()` (host_start/connection_open) then overlapped with that still-running shutdown and corrupted shared native state. Fixed with a static `SemaphoreSlim` gate in `FfiRuntimeHost` that serializes host_start/connection_open against host_shutdown process-wide, without blocking already-live connections from running concurrently. 2. rust: the newly re-enabled `test-inprocess` Windows job was canceled by a *job*-level `timeout-minutes: 20` before its own *step*-level `timeout-minutes: 60` bound was ever reached — a latent job/step timeout mismatch that was never exercised because Windows was previously excluded from this job. A cold-cache Windows Rust compile alone took longer than the job budget. Bumped both the `test` and `test-inprocess` job timeouts to accommodate their own step timeouts (100 / 70 minutes respectively). Both are genuine reliability findings directly relevant to #2525 ("[v2] Complete in-process lifecycle and platform reliability work"), caught only because this PR's changes finally exercise Windows in-process CI at all. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ream fix Real CI evidence (#2531) shows re-enabling Windows in-process E2E coverage for Rust and .NET reproducibly crashes with native memory-corruption faults (STATUS_ACCESS_VIOLATION / AccessViolationException) during ordinary connection I/O, in two independent FFI binding implementations, with no single deterministic reproducer test. This is consistent with a genuine bug in the shared native runtime cdylib (`copilot_runtime`), not something fixable from either SDK's binding code, and is out of scope for this SDK-owned issue (#2525). Filed github/copilot-agent-runtime#18990 with full reproduction evidence (stack traces, crash codes, job links) from both languages. Restores the Windows in-process exclusion for Rust's `test-inprocess` job and .NET's `test` job's non-capi in-process cells (net effect: same coverage as origin/main), replacing the stale napi-oop/SQLite-locking comments with accurate, evidence-linked ones pointing at the new upstream issue. Keeps everything else from this branch: - The bounded (10s-timeout) native host_shutdown fix across all 5 SDKs, which fixes a real, confirmed bug (unbounded synchronous shutdown calls) independent of the crash above. - The .NET native-lifecycle serializing gate (FfiRuntimeHost), a real correctness fix for an overlap between a new client's host_start and a previous client's backgrounded host_shutdown -- still valid regardless of the separate crash filed upstream. - The Rust "test" job's job-level timeout-minutes bump (20 -> 100), fixing a latent mismatch against its own 90-minute step-level timeout that could have caused a spurious cancellation independent of Windows in-process. - Node.js, Go, and Python are unaffected: their Windows in-process CI was already enabled prior to this work and continues to pass reliably (see latest CI run), so this crash appears specific to how the Rust and .NET E2E suites happen to exercise the native runtime on Windows, not the bounded-shutdown fix itself. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Rebased this lifecycle/reliability branch onto current main, which now includes CLI 1.0.84-4 and additional in-process E2E stabilization. Retried the exact Rust/.NET Windows in-process cells that previously exposed native AccessViolation/SIGSEGV crashes; the newer runtime still reproduces the blocker, so keep those cells excluded and update the workflow comments with that fresh evidence. The full retry also exposed two small Rust Windows lifecycle fixture issues unrelated to the FFI shutdown fix: the stdio job-object containment test was running under the in-process matrix, and the PID-file waiter accepted an empty file before the fixture had finished writing the child process id. Skip that stdio-only test for in-process runs and wait for a parseable PID instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

After rebasing onto the callback-reclamation cleanup from main, the Node FFI host now preserves that cleanup flow while still using Koffi's async hostShutdown path for the real runtime. Unit tests mock hostShutdown as a plain function, so fall back to the synchronous call only when the async Koffi helper is absent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

After rebasing onto the newer runtime/dependency baseline, the macOS default CAPI shard 1 was canceled while still running tests. All other .NET shards passed, so keep the diagnostic job bound but raise it from 20 to 30 minutes to avoid canceling a slow-but-healthy shard without logs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-09-11T11:42:25Z

Cross-SDK consistency review — PR #2531

This PR bounds the in-process FFI host_shutdown native call (background thread + 10s timeout) in .NET, Go, Node.js, Python, and Rust, and additionally adds a process-wide host_start/host_shutdown serialization gate in .NET (s_nativeLifecycleGate) to fix a real AccessViolationException race found on Windows CI (#2525). Tests were added consistently for all five languages.

Findings

Java SDK not updated (parity gap) — java/sdk/src/main/java/com/github/copilot/ffi/FfiRuntimeHost.java has the same in-process FFI host lifecycle shape as the other five SDKs (start() / close() → tryFinalizeCleanup()), and it still calls nativeBinding.hostShutdown(hostHandle) synchronously, inline, with no timeout (see tryFinalizeCleanup() around line 202, and the two hostShutdown calls in start()'s disposed-during-startup path around lines 123/137). This is exactly the bug this PR fixes elsewhere: a slow/stuck native shutdown (e.g. the Windows SQLite session-store case cited in [v2] Complete in-process lifecycle and platform reliability work #2525) would hang close()/dispose() and, transitively, CopilotClient.forceStop() — the documented bounded recovery path for a hung stop(). The PR's own commit message ("Bound in-process FFI host_shutdown across all SDKs...") explicitly scopes to "all five in-process SDKs," which appears to exclude Java from the outset rather than being an oversight introduced by this diff — but it does leave Java as the one remaining SDK with the original unbounded-hang defect. Worth a follow-up applying the same background-thread + timeout pattern (and ideally a matching regression test, mirroring FfiRuntimeHostTest.java's existing style) to Java for full parity.
Process-wide native lifecycle gate is .NET-only — The Windows CI investigation (commit 0ffca6a) found that bounding Dispose()'s wait let a new host's host_start/connection_open overlap with another host's still-draining host_shutdown, corrupting shared native state. The fix in .NET is a static SemaphoreSlim (s_nativeLifecycleGate) serializing these transitions process-wide. Go, Node.js, Python, and Rust all load the same underlying native library as a process-wide singleton too (Go: loadedLibrary/loadMu; Python: _loaded_library; Rust: OnceLock<...Library>; Node: shared native lib handle), but their per-host locks (e.g. Go's h.lifecycleMu) only serialize a single host's own start/shutdown, not across concurrently-live hosts in the same process. If the underlying native runtime has genuine process-wide shared state during host_start/host_shutdown (as the .NET crash suggests), the same overlap race could in principle affect the other four SDKs too, just not yet observed/reproduced there. Worth confirming whether this is .NET-specific (e.g. due to something in its native binding path) or a latent risk in the others that simply hasn't been hit by CI yet.

Everything else — the bounded-shutdown pattern, its rationale, and the regression tests — is applied consistently and idiomatically per language (goroutine+channel in Go, thread+Event in Python, std::thread+mpsc in Rust, koffi async callback in Node, Task+SemaphoreSlim in .NET). No other cross-language inconsistencies found.

Generated by SDK Consistency Review Agent for #2531 · copilot · sonnet50 · 74.4 AIC · ⌖ 12.3 AIC · ⊞ 8.3K · ◷